Add Custom OpenCV and ONNX Runtime Builds and OCR performance optimization - #57

/opencode review this pr

I've reviewed the full diff (56 files) against master. Here's my review.

Summary

Solid, well-organized PR. The KNearest removal is clean (all call sites updated, no dangling references), the hammingDistance allocation fix is the clear performance win, and the .use-based Mat lifecycle cleanup is thorough and correct — ownership transfers I checked all line up (extractor getters return fresh submat headers, maskers return owned Mats, ocrBgrMat/tensors/results are closed on all paths). The verifyModelAsset metadata cross-check and the benchmark screen are nice additions with good tests.

Issues

1. Room 3→4 migration removal can destroy user data (medium, verify) — OcrQueueDatabase.kt:26
DB stays at version = 4 but the only 3→4 path is removed and 3.json + the AutoMigrationSpec are deleted. Combined with fallbackToDestructiveMigration(dropAllTables = true), any device still on schema 3 gets its whole OCR queue DB dropped. Commit b7e9f7c asserts v3 "doesn't exist", but 3.json is checked in on master — please confirm v3 was never in a shipped release; if it was, this is a data-loss regression.

2. CI never validates the real model asset against model_info.json (medium) — ModelMetadataVerificationTest.kt
The test hardcodes expected values, so it can't catch drift between the HuggingFace model_info.json and the patched ONNX. A stale/updated asset would only surface at runtime as an ERROR status on the dependencies screen. Consider a CI step that runs verifyModelAsset against the actual downloaded assets.

3. Model verification doesn't run inference (low) — DeviceOcrOnnxHelper.kt:97-122
verifyModelAsset only creates the session and reads metadata. ORT typically reports missing kernels at Run() time, not session creation, so the "op coverage" claim is only partially validated — issues still appear on first OCR. Also, modelDecodedOutputToString (DeviceOcrOnnxHelper.kt:158) now assumes int32 output via intBuffer; the verification checks names but not the output dtype, so a dtype change would crash at runtime silently.

4. NO_OPT disables all graph optimization (low) — DeviceOcrOnnxHelper.kt:132
Necessary for the custom build, but it's an all-or-nothing knob; a comment noting the option to enable it if the custom build adds the missing optimized nodes would help future maintainers. Worth quantifying whether the ~2x queue throughput claim nets out against this on the benchmark page.

5. Double-tap race in runBenchmark() (low) — OcrPerformanceScreenViewModel.kt:98-101
if (state.running) return reads a snapshot before the coroutine sets running=true. Two rapid taps in the same frame can start concurrent benchmarks (each spawning its own session + ROI sets). Move the check inside the coroutine or gate on benchmarkJob?.isActive.

6. Soft-fail undermines the "hard-fail" intent (low) — .github/workflows/build_unstable.yml:47-49
verify-custom-libs.sh exits 1 with continue-on-error: true, so a regression back to official (larger) libs still ships. The comment in setup-custom-maven.sh calls download failure a hard-fail, but the release-side check is deliberately soft — worth confirming that's intentional and that the job-summary warning is sufficiently visible.

7. CropBlackEdges off-by-one (pre-existing, touched here) — CropBlackEdges.kt:47-68
Loops 0..width/0..height are inclusive; Rect(width, 0, 1, height) is out of bounds if the black edge runs to the last pixel. Pre-existing behavior, but these lines were rewritten — easy to fix while you're here.

Nits

• LegacyKnnModelCleanUpTask.version = 2 is odd for a brand-new task; the manager runs tasks on every launch regardless, so it's harmless but confusing.
• ImageHashesDatabase.lookupImage (ImageHashesDatabase.kt:185-187): if difference/dct throws before the try, an earlier hash Mat leaks. Trivial; could move all three into the try.
• CrnnModelStatusDetail.status() dropped the version == 0 → WARNING case; "0.0.0" now reports OK.
• OcrDependencyStatusBuilder.crnnModel: empty-string domain renders as an empty "domain: " line.

Positives

• The hammingDistance loop (ImageHashesDatabase.kt:9-21) is exactly right for a per-entry hot path — the zip/count version was the right thing to kill.
• Thorough and consistent Mat cleanup across DeviceOcr, hashers, CropBlackEdges, and the ONNX helper; I found no double-releases or use-after-release.
• collectMetadataMismatches being a pure function with JVM tests is good design.
• The benchmark correctly mirrors the production Channel-based concurrency model, uses median over mean, and validates output consistency — useful for tuning parallelCount.
• Custom-maven setup/verification scripts are well-documented and version-aware.

opencode session  |  github run